Repository navigation
feat(wallets): createServerSigner returns a sign(message) function - #2276
Open
albertoelias-crossmint wants to merge 3 commits into
Open
albertoelias-crossmint wants to merge 3 commits into
albertoelias-crossmint wants to merge 3 commits into
Conversation
A server signer made with createServerSigner returned only its address and key bytes, so a backend that had to sign an approval itself (for example a quorum member's message, which the SDK does not sign yet) rebuilt each chain's signing from keyBytes. sign(message) reuses the SDK's own EVM, Solana and Stellar server signers, so the signature matches what the SDK posts when the same server signer approves on its own. keyBytes stays intact: the adapter gets a copy, since it wipes its input.
… in createServerSigner
🦋 Changeset detectedLatest commit: 7206f94 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
[Medium impact] The PR appears safe to merge; no actionable issues were found. Reviews (1) · Last reviewed commit: "test(wallets): cover a non-base58 Solana..." · Reviewed by Greptile |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Problem
A backend that holds a server signer secret can derive its address with
createServerSigner(), but cannot sign with it. The function returns{ type, address, keyBytes }and nothing else. When the API asks that signer for an approval (a transaction or signature approval, or a quorum member's approval message), the developer has to rebuild the signing per chain fromkeyBytes: viemsignMessagewith{ raw }on EVM, ed25519 over the base58 message on Solana, ed25519 over the base64-decoded message with base64 out on Stellar. Callingsigner.sign(message)fails withTypeError: signer.sign is not a function, and a wrong rebuild is only caught when the API rejects the approval.What this PR does
createServerSigner()also returnssign(message). It delegates to the sameEVMServerSigner,SolanaServerSignerandStellarServerSignerthe SDK uses when a server signer approves on its own, so the output is what the API already accepts.keyBytesis left intact: the adapter wipes the bytes it is given, so it gets a copy.flowchart LR subgraph Before A1["createServerSigner()"] --> B1["address + keyBytes"] --> C1["caller re-implements per-chain signing"] end subgraph After A2["createServerSigner()"] --> B2["address + keyBytes + sign"] --> C2["sign(pending.message)"] --> D2["POST under server:address"] endResult
signer.sign(message)TypeError: signer.sign is not a functionUnchanged:
address,keyBytes,CreateServerSignerParams.ServerSignergains a requiredsignfield; code that only reads the existing fields is unaffected, code that constructs aServerSignerliteral by hand would need to add it.What to check when reviewing
utils/server-signers/createServerSigner.ts: reuse ofassembleServerSigner, and thekeyBytes.slice()copy.Buffer).signMessage, insidewallet.approve, and adds no standalonesignhelper, so this does not duplicate it. If feat(wallets): approve as a quorum member (EVM + Stellar quorum E1) #2255's wallet-level path covers a caller's need,signis still the way to sign outside a wallet. Samewallet.tsstack as feat(wallets): read quorum signer entries (quorum foundation 1/3) #2214, feat(wallets): approval collection and poll outcome for quorum approvals (quorum foundation 4/4) #2225, feat(wallets): acting-signer seam, held signers and quorum API errors at call sites (quorum foundation 3/4) #2233, feat(wallets): a member-only key acts as its quorum (EVM + Stellar quorum E2) #2256, feat(wallets): signer changes approved by a recovery quorum (EVM quorum E3) #2261, feat(wallets): a quorum acting signer returns the created transaction unsigned, as prepareOnly does (quorum foundation 5/5) #2268, but this PR touches none of those files.Test plan
pnpm --filter @crossmint/wallets-sdk test:vitest: 770 tests pass.src/api/client.test.tsneedsPREVIEW_CK_KEYand was not run locally; unrelated.createServerSigner.test.ts(8): EVM signature recovers to the address; Solana and Stellar signatures verify against the address's key; EVM, Solana and Stellar reject malformed messages; repeated signing works;keyBytesunchanged. All 8 fail against the old source.pnpm biome checkand the package build pass.Package updates
@crossmint/wallets-sdkminor changeset (.changeset/server-signer-sign.md)Checklist
Rollback
Revert the commit;
signis additive and nothing else reads it.Closes WAL-13399
🤖 Generated with Claude Code